[HVM] Save/restore: clean up marshalling code
authorTim Deegan <Tim.Deegan@xensource.com>
Wed, 31 Jan 2007 10:27:10 +0000 (10:27 +0000)
committerTim Deegan <Tim.Deegan@xensource.com>
Wed, 31 Jan 2007 10:27:10 +0000 (10:27 +0000)
- All entries are now defined as structs and saved/restored
  in self-contained operations.
- Save/restore operations are type-safe, to tie each entry's
  typecode to a particular struct and its length.
- Save/restore handlers are registered once per host instead of
  per domain.
- Detect buffer overrun before it happens and abort.

Signed-off-by: Tim Deegan <Tim.Deegan@xensource.com>
13 files changed:
xen/arch/x86/domctl.c
xen/arch/x86/hvm/hvm.c
xen/arch/x86/hvm/i8254.c
xen/arch/x86/hvm/intercept.c
xen/arch/x86/hvm/svm/svm.c
xen/arch/x86/hvm/vioapic.c
xen/arch/x86/hvm/vlapic.c
xen/arch/x86/hvm/vmx/vmx.c
xen/arch/x86/hvm/vpic.c
xen/include/asm-x86/hvm/domain.h
xen/include/asm-x86/hvm/hvm.h
xen/include/asm-x86/hvm/support.h
xen/include/public/hvm/save.h

index fbf55fed5b4a2bf74f21fb6cf767e88fd8225d75..db18c7cecfef6a3a237199a4b64586c6c86cf0ac 100644 (file)
@@ -302,6 +302,8 @@ long arch_do_domctl(
         ret = -EFAULT;
         if ( copy_from_guest(c, domctl->u.hvmcontext.ctxt, 1) != 0 )
             goto sethvmcontext_out;
+        c->size = sizeof (c->data);
+        c->cur = 0;
 
         ret = -EINVAL;
         if ( !is_hvm_domain(d) ) 
@@ -330,6 +332,7 @@ long arch_do_domctl(
         if ( (c = xmalloc(struct hvm_domain_context)) == NULL )
             goto gethvmcontext_out;
         memset(c, 0, sizeof(*c));
+        c->size = sizeof (c->data);
         
         ret = -ENODATA;
         if ( !is_hvm_domain(d) ) 
index f0f9caec37b2289c705447185a53de83b4e6b65e..aaf27c0aee2f58277bdd9215fc768763f74632f7 100644 (file)
@@ -168,19 +168,11 @@ int hvm_domain_initialise(struct domain *d)
 
 void hvm_domain_destroy(struct domain *d)
 {
-    HVMStateEntry *se, *dse;
     pit_deinit(d);
     rtc_deinit(d);
     pmtimer_deinit(d);
     hpet_deinit(d);
 
-    se = d->arch.hvm_domain.first_se;
-    while (se) {
-        dse = se;
-        se = se->next;
-        xfree(dse);
-    }
     if ( d->arch.hvm_domain.shared_page_va )
         unmap_domain_page_global(
             (void *)d->arch.hvm_domain.shared_page_va);
@@ -189,23 +181,43 @@ void hvm_domain_destroy(struct domain *d)
         unmap_domain_page_global((void *)d->arch.hvm_domain.buffered_io_va);
 }
 
-void hvm_save_cpu_ctxt(hvm_domain_context_t *h, void *opaque)
+static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 {
-    struct vcpu *v = opaque;
-
-    /* We don't need to save state for a vcpu that is down; the restore 
-     * code will leave it down if there is nothing saved. */
-    if ( test_bit(_VCPUF_down, &v->vcpu_flags) ) 
-        return;
+    struct vcpu *v;
+    struct hvm_hw_cpu ctxt;
 
-    hvm_funcs.save_cpu_ctxt(h, opaque);
+    for_each_vcpu(d, v)
+    {
+        /* We don't need to save state for a vcpu that is down; the restore 
+         * code will leave it down if there is nothing saved. */
+        if ( test_bit(_VCPUF_down, &v->vcpu_flags) ) 
+            continue;
+
+        hvm_funcs.save_cpu_ctxt(v, &ctxt);
+        if ( hvm_save_entry(CPU, v->vcpu_id, h, &ctxt) != 0 )
+            return 1; 
+    }
+    return 0;
 }
 
-int hvm_load_cpu_ctxt(hvm_domain_context_t *h, void *opaque, int version)
+static int hvm_load_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 {
-    struct vcpu *v = opaque;
+    int vcpuid;
+    struct vcpu *v;
+    struct hvm_hw_cpu ctxt;
+
+    /* Which vcpu is this? */
+    vcpuid = hvm_load_instance(h);
+    if ( vcpuid > MAX_VIRT_CPUS || (v = d->vcpu[vcpuid]) == NULL ) 
+    {
+        gdprintk(XENLOG_ERR, "HVM restore: domain has no vcpu %u\n", vcpuid);
+        return -EINVAL;
+    }
+
+    if ( hvm_load_entry(CPU, h, &ctxt) != 0 ) 
+        return -EINVAL;
 
-    if ( hvm_funcs.load_cpu_ctxt(h, opaque, version) < 0 )
+    if ( hvm_funcs.load_cpu_ctxt(v, &ctxt) < 0 )
         return -EINVAL;
 
     /* Auxiliary processors should be woken immediately. */
@@ -215,14 +227,12 @@ int hvm_load_cpu_ctxt(hvm_domain_context_t *h, void *opaque, int version)
     return 0;
 }
 
+HVM_REGISTER_SAVE_RESTORE(CPU, hvm_save_cpu_ctxt, hvm_load_cpu_ctxt);
+
 int hvm_vcpu_initialise(struct vcpu *v)
 {
     int rc;
 
-    hvm_register_savevm(v->domain, "xen_hvm_cpu", v->vcpu_id, 1,
-                        hvm_save_cpu_ctxt, hvm_load_cpu_ctxt, 
-                        (void *)v);
-
     if ( (rc = vlapic_init(v)) != 0 )
         return rc;
 
@@ -248,9 +258,6 @@ int hvm_vcpu_initialise(struct vcpu *v)
     pmtimer_init(v, ACPI_PM_TMR_BLK_ADDRESS);
     hpet_init(v);
  
-    /* init hvm sharepage */
-    shpage_init(v->domain, get_sp(v->domain));
-
     /* Init guest TSC to start from zero. */
     hvm_set_guest_time(v, 0);
 
index 2fe8ad5f5f4811b464f3652694c0f2455d22a20a..5473f914f4ab41525ea0ae5674ab4028cd04b60d 100644 (file)
@@ -411,28 +411,24 @@ static void pit_info(PITState *pit)
 }
 #endif
 
-static void pit_save(hvm_domain_context_t *h, void *opaque)
+static int pit_save(struct domain *d, hvm_domain_context_t *h)
 {
-    struct domain *d = opaque;
     PITState *pit = &d->arch.hvm_domain.pl_time.vpit;
     
     pit_info(pit);
 
     /* Save the PIT hardware state */
-    hvm_put_struct(h, &pit->hw);
+    return hvm_save_entry(PIT, 0, h, &pit->hw);
 }
 
-static int pit_load(hvm_domain_context_t *h, void *opaque, int version_id)
+static int pit_load(struct domain *d, hvm_domain_context_t *h)
 {
-    struct domain *d = opaque;
     PITState *pit = &d->arch.hvm_domain.pl_time.vpit;
     int i;
 
-    if (version_id != 1)
-        return -EINVAL;
-
     /* Restore the PIT hardware state */
-    hvm_get_struct(h, &pit->hw);
+    if ( hvm_load_entry(PIT, h, &pit->hw) )
+        return 1;
     
     /* Recreate platform timers from hardware state.  There will be some 
      * time jitter here, but the wall-clock will have jumped massively, so 
@@ -447,6 +443,8 @@ static int pit_load(hvm_domain_context_t *h, void *opaque, int version_id)
     return 0;
 }
 
+HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load);
+
 static void pit_reset(void *opaque)
 {
     PITState *pit = opaque;
@@ -474,7 +472,6 @@ void pit_init(struct vcpu *v, unsigned long cpu_khz)
     pt++; pt->vcpu = v;
     pt++; pt->vcpu = v;
 
-    hvm_register_savevm(v->domain, "xen_hvm_i8254", PIT_BASE, 1, pit_save, pit_load, v->domain);
     register_portio_handler(v->domain, PIT_BASE, 4, handle_pit_io);
     /* register the speaker port */
     register_portio_handler(v->domain, 0x61, 1, handle_speaker_io);
index c41a937b03ada50ddfaaea5fda5b883eb0bb654f..d5a9393c1d67ee0f5afefd08c8b036b29875082e 100644 (file)
@@ -157,287 +157,174 @@ static inline void hvm_mmio_access(struct vcpu *v,
     }
 }
 
-
-int hvm_register_savevm(struct domain *d,
-                    const char *idstr,
-                    int instance_id,
-                    int version_id,
-                    SaveStateHandler *save_state,
-                    LoadStateHandler *load_state,
-                    void *opaque)
+/* List of handlers for various HVM save and restore types */
+static struct { 
+    hvm_save_handler save;
+    hvm_load_handler load; 
+} hvm_sr_handlers [HVM_SAVE_CODE_MAX + 1] = {{NULL, NULL},};
+
+/* Init-time function to add entries to that list */
+void hvm_register_savevm(uint16_t typecode, 
+                         hvm_save_handler save_state,
+                         hvm_load_handler load_state)
 {
-    HVMStateEntry *se, **pse;
-
-    if ( (se = xmalloc(struct HVMStateEntry)) == NULL ){
-        printk("allocat hvmstate entry fail.\n");
-        return -1;
-    }
-
-    safe_strcpy(se->idstr, idstr);
-
-    se->instance_id = instance_id;
-    se->version_id = version_id;
-    se->save_state = save_state;
-    se->load_state = load_state;
-    se->opaque = opaque;
-    se->next = NULL;
-
-    /* add at the end of list */
-    pse = &d->arch.hvm_domain.first_se;
-    while (*pse != NULL)
-        pse = &(*pse)->next;
-    *pse = se;
-    return 0;
+    ASSERT(typecode <= HVM_SAVE_CODE_MAX);
+    ASSERT(hvm_sr_handlers[typecode].save == NULL);
+    ASSERT(hvm_sr_handlers[typecode].load == NULL);
+    hvm_sr_handlers[typecode].save = save_state;
+    hvm_sr_handlers[typecode].load = load_state;
 }
 
+
 int hvm_save(struct domain *d, hvm_domain_context_t *h)
 {
-    uint32_t len, len_pos, cur_pos;
     uint32_t eax, ebx, ecx, edx;
-    HVMStateEntry *se;
-    char *chgset;
+    char *c;
     struct hvm_save_header hdr;
+    struct hvm_save_end end;
+    hvm_save_handler handler;
+    uint16_t i;
 
     hdr.magic = HVM_FILE_MAGIC;
     hdr.version = HVM_FILE_VERSION;
+
+    /* Save some CPUID bits */
     cpuid(1, &eax, &ebx, &ecx, &edx);
     hdr.cpuid = eax;
-    hvm_put_struct(h, &hdr);
 
-    /* save xen changeset */
-    chgset = strrchr(XEN_CHANGESET, ' ');
-    if ( chgset )
-        chgset++;
-    else
-        chgset = XEN_CHANGESET;
-
-    len = strlen(chgset);
-    hvm_put_8u(h, len);
-    hvm_put_buffer(h, chgset, len);
-
-    for(se = d->arch.hvm_domain.first_se; se != NULL; se = se->next) {
-        /* ID string */
-        len = strnlen(se->idstr, sizeof(se->idstr));
-        hvm_put_8u(h, len);
-        hvm_put_buffer(h, se->idstr, len);
-
-        hvm_put_32u(h, se->instance_id);
-        hvm_put_32u(h, se->version_id);
-
-        /* record size */
-        len_pos = hvm_ctxt_tell(h);
-        hvm_put_32u(h, 0);
+    /* Save xen changeset */
+    c = strrchr(XEN_CHANGESET, ':');
+    if ( c )
+        hdr.changeset = simple_strtoll(c, NULL, 16);
+    else 
+        hdr.changeset = -1ULL; /* Unknown */
 
-        se->save_state(h, se->opaque);
-
-        cur_pos = hvm_ctxt_tell(h);
-        len = cur_pos - len_pos - 4;
-        hvm_ctxt_seek(h, len_pos);
-        hvm_put_32u(h, len);
-        hvm_ctxt_seek(h, cur_pos);
+    if ( hvm_save_entry(HEADER, 0, h, &hdr) != 0 )
+    {
+        gdprintk(XENLOG_ERR, "HVM save: failed to write header\n");
+        return -1;
+    } 
 
+    /* Save all available kinds of state */
+    for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ ) 
+    {
+        handler = hvm_sr_handlers[i].save;
+        if ( handler != NULL ) 
+        {
+            if ( handler(d, h) != 0 ) 
+            {
+                gdprintk(XENLOG_ERR, 
+                         "HVM save: failed to save type %"PRIu16"\n", i);
+                return -1;
+            } 
+        }
     }
 
-    h->size = hvm_ctxt_tell(h);
-    hvm_ctxt_seek(h, 0);
-
-    if (h->size >= HVM_CTXT_SIZE) {
-        printk("hvm_domain_context overflow when hvm_save! need %"PRId32" bytes for use.\n", h->size);
+    /* Save an end-of-file marker */
+    if ( hvm_save_entry(END, 0, h, &end) != 0 )
+    {
+        /* Run out of data */
+        gdprintk(XENLOG_ERR, "HVM save: no room for end marker.\n");
         return -1;
     }
 
+    /* Save macros should not have let us overrun */
+    ASSERT(h->cur <= h->size);
     return 0;
-
-}
-
-static HVMStateEntry *find_se(struct domain *d, const char *idstr, int instance_id)
-{
-    HVMStateEntry *se;
-
-    for(se = d->arch.hvm_domain.first_se; se != NULL; se = se->next) {
-        if (!strncmp(se->idstr, idstr, sizeof(se->idstr)) &&
-            instance_id == se->instance_id){
-            return se;
-        }
-    }
-    return NULL;
 }
 
 int hvm_load(struct domain *d, hvm_domain_context_t *h)
 {
-    uint32_t len, rec_len, rec_pos, instance_id, version_id;
     uint32_t eax, ebx, ecx, edx;
-    HVMStateEntry *se;
-    char idstr[HVM_SE_IDSTR_LEN];
-    xen_changeset_info_t chgset;
-    char *cur_chgset;
-    int ret;
+    char *c;
+    uint64_t cset;
     struct hvm_save_header hdr;
+    struct hvm_save_descriptor *desc;
+    hvm_load_handler handler;
     struct vcpu *v;
-
-    if (h->size >= HVM_CTXT_SIZE) {
-        printk("hvm_load fail! seems hvm_domain_context overflow when hvm_save! need %"PRId32" bytes.\n", h->size);
+    
+    /* Read the save header, which must be first */
+    if ( hvm_load_entry(HEADER, h, &hdr) != 0 ) 
         return -1;
-    }
-
-    hvm_ctxt_seek(h, 0);
-
-    hvm_get_struct(h, &hdr);
 
     if (hdr.magic != HVM_FILE_MAGIC) {
-        printk("HVM restore magic dismatch!\n");
+        gdprintk(XENLOG_ERR, 
+                 "HVM restore: bad magic number %#"PRIx32"\n", hdr.magic);
         return -1;
     }
 
     if (hdr.version != HVM_FILE_VERSION) {
-        printk("HVM restore version dismatch!\n");
+        gdprintk(XENLOG_ERR, 
+                 "HVM restore: unsupported version %u\n", hdr.version);
         return -1;
     }
 
-    /* check cpuid */
     cpuid(1, &eax, &ebx, &ecx, &edx);
-    /*TODO: need difine how big difference is acceptable */
+    /*TODO: need to define how big a difference is acceptable */
     if (hdr.cpuid != eax)
-        printk("warnings: try to restore hvm guest(0x%"PRIx32") "
-               "on a different type processor(0x%"PRIx32").\n",
-                hdr.cpuid,
-                eax);
+        gdprintk(XENLOG_WARNING, "HVM restore: saved CPUID (%#"PRIx32") "
+               "does not match host (%#"PRIx32").\n", hdr.cpuid, eax);
 
 
-    /* check xen change set */
-    cur_chgset = strrchr(XEN_CHANGESET, ' ');
-    if ( cur_chgset )
-        cur_chgset++;
+    c = strrchr(XEN_CHANGESET, ':');
+    if ( hdr.changeset == -1ULL )
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: Xen changeset was not saved.\n");
+    else if ( c == NULL )
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: Xen changeset is not available.\n");
     else
-        cur_chgset = XEN_CHANGESET;
-
-    len = hvm_get_8u(h);
-    if (len > 20) { /*typical length is 18 -- "revision number:changeset id" */
-        printk("wrong change set length %d when hvm restore!\n", len);
-        return -1;
+    {
+        cset = simple_strtoll(c, NULL, 16);
+        if ( hdr.changeset != cset )
+        gdprintk(XENLOG_WARNING, "HVM restore: saved Xen changeset (%#"PRIx64
+                 ") does not match host (%#"PRIx64").\n", hdr.changeset, cset);
     }
 
-    hvm_get_buffer(h, chgset, len);
-    chgset[len] = '\0';
-    if (strncmp(cur_chgset, chgset, len + 1))
-        printk("warnings: try to restore hvm guest(%s) on a different changeset %s.\n",
-                chgset, cur_chgset);
-
-
-    if ( !strcmp(cur_chgset, "unavailable") )
-        printk("warnings: try to restore hvm guest when changeset is unavailable.\n");
-
-
     /* Down all the vcpus: we only re-enable the ones that had state saved. */
     for_each_vcpu(d, v) 
         if ( test_and_set_bit(_VCPUF_down, &v->vcpu_flags) )
             vcpu_sleep_nosync(v);
 
     while(1) {
-        if (hvm_ctxt_end(h)) {
-            break;
-        }
 
-        /* ID string */
-        len = hvm_get_8u(h);
-        if (len > HVM_SE_IDSTR_LEN) {
-            printk("wrong HVM save entry idstr len %d!", len);
+        if ( h->size - h->cur < sizeof(struct hvm_save_descriptor) )
+        {
+            /* Run out of data */
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: save did not end with a null entry\n");
             return -1;
         }
-
-        hvm_get_buffer(h, idstr, len);
-        idstr[len] = '\0';
-
-        instance_id = hvm_get_32u(h);
-        version_id = hvm_get_32u(h);
-
-        printk("HVM S/R Loading \"%s\" instance %#x\n", idstr, instance_id);
-
-        rec_len = hvm_get_32u(h);
-        rec_pos = hvm_ctxt_tell(h);
-
-        se = find_se(d, idstr, instance_id);
-        if (se == NULL) {
-            printk("warnings: hvm load can't find device %s's instance %d!\n",
-                    idstr, instance_id);
-        } else {
-            ret = se->load_state(h, se->opaque, version_id);
-            if (ret < 0)
-                printk("warnings: loading state fail for device %s instance %d!\n",
-                        idstr, instance_id);
+        
+        /* Read the typecode of the next entry  and check for the end-marker */
+        desc = (struct hvm_save_descriptor *)(&h->data[h->cur]);
+        if ( desc->typecode == 0 )
+            return 0; 
+        
+        /* Find the handler for this entry */
+        if ( desc->typecode > HVM_SAVE_CODE_MAX 
+             || (handler = hvm_sr_handlers[desc->typecode].load) == NULL ) 
+        {
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: unknown entry typecode %u\n", 
+                     desc->typecode);
+            return -1;
         }
-                    
 
-        /* make sure to jump end of record */
-        if ( hvm_ctxt_tell(h) - rec_pos != rec_len) {
-            printk("wrong hvm record size, maybe some dismatch between save&restore handler!\n");
+        /* Load the entry */
+        if ( handler(d, h) != 0 ) 
+        {
+            gdprintk(XENLOG_ERR, 
+                     "HVM restore: failed to load entry %u/%u\n", 
+                     desc->typecode, desc->instance);
+            return -1;
         }
-        hvm_ctxt_seek(h, rec_pos + rec_len);
     }
 
-    return 0;
+    /* Not reached */
 }
 
 
-#ifdef HVM_DEBUG_SUSPEND
-static void shpage_info(shared_iopage_t *sh)
-{
-
-    vcpu_iodata_t *p = &sh->vcpu_iodata[0];
-    ioreq_t *req = &p->vp_ioreq;
-    printk("*****sharepage_info******!\n");
-    printk("vp_eport=%d\n", p->vp_eport);
-    printk("io packet: "
-                     "state:%x, pvalid: %x, dir:%x, port: %"PRIx64", "
-                     "data: %"PRIx64", count: %"PRIx64", size: %"PRIx64"\n",
-                     req->state, req->data_is_ptr, req->dir, req->addr,
-                     req->data, req->count, req->size);
-}
-#else
-static void shpage_info(shared_iopage_t *sh)
-{
-}
-#endif
-
-static void shpage_save(hvm_domain_context_t *h, void *opaque)
-{
-    /* XXX:no action required for shpage save/restore, since it's in guest memory
-     * keep it for debug purpose only */
-
-#if 0
-    struct shared_iopage *s = opaque;
-    /* XXX:smp */
-    struct ioreq *req = &s->vcpu_iodata[0].vp_ioreq;
-    
-    shpage_info(s);
-
-    hvm_put_buffer(h, (char*)req, sizeof(struct ioreq));
-#endif
-}
-
-static int shpage_load(hvm_domain_context_t *h, void *opaque, int version_id)
-{
-    struct shared_iopage *s = opaque;
-#if 0
-    /* XXX:smp */
-    struct ioreq *req = &s->vcpu_iodata[0].vp_ioreq;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    hvm_get_buffer(h, (char*)req, sizeof(struct ioreq));
-
-
-#endif
-    shpage_info(s);
-    return 0;
-}
-
-void shpage_init(struct domain *d, shared_iopage_t *sp)
-{
-    hvm_register_savevm(d, "xen_hvm_shpage", 0x10, 1, shpage_save, shpage_load, sp);
-}
-
 int hvm_buffered_io_intercept(ioreq_t *p)
 {
     struct vcpu *v = current;
index ff2d46896ffafc48900192018aad36113b34ad6d..b2637a78076f76e9f64d66adb0f1459734f16f3d 100644 (file)
@@ -603,29 +603,16 @@ void svm_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
     // dump_msr_state(guest_state);
 }
 
-void svm_save_vmcb_ctxt(hvm_domain_context_t *h, void *opaque)
+void svm_save_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
 {
-    struct vcpu *v = opaque;
-    struct hvm_hw_cpu ctxt;
-
-    svm_save_cpu_state(v, &ctxt);
-
-    svm_vmcs_save(v, &ctxt);
-
-    hvm_put_struct(h, &ctxt);
+    svm_save_cpu_state(v, ctxt);
+    svm_vmcs_save(v, ctxt);
 }
 
-int svm_load_vmcb_ctxt(hvm_domain_context_t *h, void *opaque, int version)
+int svm_load_vmcb_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
 {
-    struct vcpu *v = opaque;
-    struct hvm_hw_cpu ctxt;
-
-    if (version != 1)
-        return -EINVAL;
-
-    hvm_get_struct(h, &ctxt);
-    svm_load_cpu_state(v, &ctxt);
-    if (svm_vmcb_restore(v, &ctxt)) {
+    svm_load_cpu_state(v, ctxt);
+    if (svm_vmcb_restore(v, ctxt)) {
         printk("svm_vmcb restore failed!\n");
         domain_crash(v->domain);
         return -EINVAL;
index 3efe1286cdb7c0a16f9a110915c23f5b0e621385..d43eced7407b9ea65c7918b334863aa8215c49d8 100644 (file)
@@ -523,50 +523,58 @@ static void hvmirq_info(struct hvm_hw_irq *hvm_irq)
 }
 #endif
 
-static void ioapic_save(hvm_domain_context_t *h, void *opaque)
+
+static int ioapic_save(struct domain *d, hvm_domain_context_t *h)
 {
-    struct domain *d = opaque;
     struct hvm_hw_vioapic *s = domain_vioapic(d);
-    struct hvm_hw_irq *hvm_irq = &d->arch.hvm_domain.irq;
-
     ioapic_info(s);
-    hvmirq_info(hvm_irq);
 
     /* save io-apic state*/
-    hvm_put_struct(h, s);
+    return ( hvm_save_entry(IOAPIC, 0, h, s) );
+}
+
+static int ioapic_save_irqs(struct domain *d, hvm_domain_context_t *h)
+{
+    struct hvm_hw_irq *hvm_irq = &d->arch.hvm_domain.irq;
+    hvmirq_info(hvm_irq);
 
-    /* save hvm irq state */
-    hvm_put_struct(h, hvm_irq);
+    /* save IRQ state*/
+    return ( hvm_save_entry(IRQ, 0, h, hvm_irq) );    
 }
 
-static int ioapic_load(hvm_domain_context_t *h, void *opaque, int version_id)
+
+static int ioapic_load(struct domain *d, hvm_domain_context_t *h)
 {
-    struct domain *d = opaque;
     struct hvm_hw_vioapic *s = domain_vioapic(d);
-    struct hvm_hw_irq *hvm_irq = &d->arch.hvm_domain.irq;
     
-    if (version_id != 1)
+    /* restore ioapic state */
+    if ( hvm_load_entry(IOAPIC, h, s) != 0 )
         return -EINVAL;
 
-    /* restore ioapic state */
-    hvm_get_struct(h, s);
+    ioapic_info(s);
+    return 0;
+}
+
+static int ioapic_load_irqs(struct domain *d, hvm_domain_context_t *h)
+{
+    struct hvm_hw_irq *hvm_irq = &d->arch.hvm_domain.irq;
 
     /* restore irq state */
-    hvm_get_struct(h, hvm_irq);
+    if ( hvm_load_entry(IRQ, h, hvm_irq) != 0 )
+        return -EINVAL;
 
-    ioapic_info(s);
     hvmirq_info(hvm_irq);
-
     return 0;
 }
 
+HVM_REGISTER_SAVE_RESTORE(IOAPIC, ioapic_save, ioapic_load);
+HVM_REGISTER_SAVE_RESTORE(IRQ, ioapic_save_irqs, ioapic_load_irqs);
+
 void vioapic_init(struct domain *d)
 {
     struct hvm_hw_vioapic *vioapic = domain_vioapic(d);
     int i;
 
-    hvm_register_savevm(d, "xen_hvm_ioapic", 0, 1, ioapic_save, ioapic_load, d);
-
     memset(vioapic, 0, sizeof(*vioapic));
     for ( i = 0; i < VIOAPIC_NUM_PINS; i++ )
         vioapic->redirtbl[i].fields.mask = 1;
index 1951dbb2e6dafbf73f5ebe2d0b6e121cb6f78588..4ac13ba9c45341a561813867033e646c16e33ca5 100644 (file)
@@ -810,51 +810,106 @@ static void lapic_info(struct vlapic *s)
 }
 #endif
 
-static void lapic_save(hvm_domain_context_t *h, void *opaque)
+/* rearm the actimer if needed, after a HVM restore */
+static void lapic_rearm(struct vlapic *s)
 {
-    struct vlapic *s = opaque;
-
-    lapic_info(s);
-
-    hvm_put_struct(h, &s->hw);
-    hvm_put_struct(h, s->regs);
-}
-
-static int lapic_load(hvm_domain_context_t *h, void *opaque, int version_id)
-{
-    struct vlapic *s = opaque;
-    struct vcpu *v = vlapic_vcpu(s);
     unsigned long tmict;
 
-    if (version_id != 1)
-        return -EINVAL;
-
-    hvm_get_struct(h, &s->hw);
-    hvm_get_struct(h, s->regs);
-
-    /* rearm the actiemr if needed */
     tmict = vlapic_get_reg(s, APIC_TMICT);
     if (tmict > 0) {
         uint64_t period = APIC_BUS_CYCLE_NS * (uint32_t)tmict * s->hw.timer_divisor;
         uint32_t lvtt = vlapic_get_reg(s, APIC_LVTT);
 
         s->pt.irq = lvtt & APIC_VECTOR_MASK;
-        create_periodic_time(v, &s->pt, period, s->pt.irq,
+        create_periodic_time(vlapic_vcpu(s), &s->pt, period, s->pt.irq,
                              vlapic_lvtt_period(s), NULL, s);
 
         printk("lapic_load to rearm the actimer:"
                     "bus cycle is %uns, "
                     "saved tmict count %lu, period %"PRIu64"ns, irq=%"PRIu8"\n",
                     APIC_BUS_CYCLE_NS, tmict, period, s->pt.irq);
+    }
+
+    lapic_info(s);
+}
+
+static int lapic_save_hidden(struct domain *d, hvm_domain_context_t *h)
+{
+    struct vcpu *v;
+    struct vlapic *s;
+
+    for_each_vcpu(d, v)
+    {
+        s = vcpu_vlapic(v);
+        lapic_info(s);
+
+        if ( hvm_save_entry(LAPIC, v->vcpu_id, h, &s->hw) != 0 )
+            return 1; 
+    }
+    return 0;
+}
 
+static int lapic_save_regs(struct domain *d, hvm_domain_context_t *h)
+{
+    struct vcpu *v;
+    struct vlapic *s;
+
+    for_each_vcpu(d, v)
+    {
+        s = vcpu_vlapic(v);
+        if ( hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs) != 0 )
+            return 1; 
     }
+    return 0;
+}
 
+static int lapic_load_hidden(struct domain *d, hvm_domain_context_t *h)
+{
+    uint16_t vcpuid;
+    struct vcpu *v;
+    struct vlapic *s;
+    
+    /* Which vlapic to load? */
+    vcpuid = hvm_load_instance(h); 
+    if ( vcpuid > MAX_VIRT_CPUS || (v = d->vcpu[vcpuid]) == NULL ) 
+    {
+        gdprintk(XENLOG_ERR, "HVM restore: domain has no vlapic %u\n", vcpuid);
+        return -EINVAL;
+    }
+    s = vcpu_vlapic(v);
+    
+    if ( hvm_load_entry(LAPIC, h, &s->hw) != 0 ) 
+        return -EINVAL;
 
     lapic_info(s);
+    return 0;
+}
 
+static int lapic_load_regs(struct domain *d, hvm_domain_context_t *h)
+{
+    uint16_t vcpuid;
+    struct vcpu *v;
+    struct vlapic *s;
+    
+    /* Which vlapic to load? */
+    vcpuid = hvm_load_instance(h); 
+    if ( vcpuid > MAX_VIRT_CPUS || (v = d->vcpu[vcpuid]) == NULL ) 
+    {
+        gdprintk(XENLOG_ERR, "HVM restore: domain has no vlapic %u\n", vcpuid);
+        return -EINVAL;
+    }
+    s = vcpu_vlapic(v);
+    
+    if ( hvm_load_entry(LAPIC_REGS, h, s->regs) != 0 ) 
+        return -EINVAL;
+
+    lapic_rearm(s);
     return 0;
 }
 
+HVM_REGISTER_SAVE_RESTORE(LAPIC, lapic_save_hidden, lapic_load_hidden);
+HVM_REGISTER_SAVE_RESTORE(LAPIC_REGS, lapic_save_regs, lapic_load_regs);
+
 int vlapic_init(struct vcpu *v)
 {
     struct vlapic *vlapic = vcpu_vlapic(v);
@@ -873,7 +928,6 @@ int vlapic_init(struct vcpu *v)
     vlapic->regs = map_domain_page_global(page_to_mfn(vlapic->regs_page));
     memset(vlapic->regs, 0, PAGE_SIZE);
 
-    hvm_register_savevm(v->domain, "xen_hvm_lapic", v->vcpu_id, 1, lapic_save, lapic_load, vlapic);
     vlapic_reset(vlapic);
 
     vlapic->hw.apic_base_msr = MSR_IA32_APICBASE_ENABLE | APIC_DEFAULT_PHYS_BASE;
index f203ed9107750d7d9682c33163358d71052eb3f3..c3a2ed4585b0d6c532bb6e84f271145689b7250d 100644 (file)
@@ -364,19 +364,9 @@ static inline void __restore_debug_registers(struct vcpu *v)
     /* DR7 is loaded from the VMCS. */
 }
 
-static int __get_instruction_length(void);
 int vmx_vmcs_save(struct vcpu *v, struct hvm_hw_cpu *c)
-{
-    unsigned long inst_len;
-
-    inst_len = __get_instruction_length();
+{    
     c->eip = __vmread(GUEST_RIP);
-
-#ifdef HVM_DEBUG_SUSPEND
-    printk("vmx_vmcs_save: inst_len=0x%lx, eip=0x%"PRIx64".\n", 
-            inst_len, c->eip);
-#endif
-
     c->esp = __vmread(GUEST_RSP);
     c->eflags = __vmread(GUEST_RFLAGS);
 
@@ -632,30 +622,18 @@ void vmx_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
 }
 
 
-void vmx_save_vmcs_ctxt(hvm_domain_context_t *h, void *opaque)
+void vmx_save_vmcs_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
 {
-    struct vcpu *v = opaque;
-    struct hvm_hw_cpu ctxt;
-
-    vmx_save_cpu_state(v, &ctxt);
+    vmx_save_cpu_state(v, ctxt);
     vmx_vmcs_enter(v);
-    vmx_vmcs_save(v, &ctxt);
+    vmx_vmcs_save(v, ctxt);
     vmx_vmcs_exit(v);
-
-    hvm_put_struct(h, &ctxt);
 }
 
-int vmx_load_vmcs_ctxt(hvm_domain_context_t *h, void *opaque, int version)
+int vmx_load_vmcs_ctxt(struct vcpu *v, struct hvm_hw_cpu *ctxt)
 {
-    struct vcpu *v = opaque;
-    struct hvm_hw_cpu ctxt;
-
-    if (version != 1)
-        return -EINVAL;
-
-    hvm_get_struct(h, &ctxt);
-    vmx_load_cpu_state(v, &ctxt);
-    if (vmx_vmcs_restore(v, &ctxt)) {
+    vmx_load_cpu_state(v, ctxt);
+    if (vmx_vmcs_restore(v, ctxt)) {
         printk("vmx_vmcs restore failed!\n");
         domain_crash(v->domain);
         return -EINVAL;
index 290b5f3129622d95a7896a2ececb96a723b70d89..41e3ed11a489b6cf3370901eeeb36e9b9b366fcb 100644 (file)
@@ -404,27 +404,44 @@ static void vpic_info(struct hvm_hw_vpic *s)
 }
 #endif
 
-static void vpic_save(hvm_domain_context_t *h, void *opaque)
+static int vpic_save(struct domain *d, hvm_domain_context_t *h)
 {
-    struct hvm_hw_vpic *s = opaque;
-    
-    vpic_info(s);
-    hvm_put_struct(h, s);
+    struct hvm_hw_vpic *s;
+    int i;
+
+    /* Save the state of both PICs */
+    for ( i = 0; i < 2 ; i++ )
+    {
+        s = &d->arch.hvm_domain.vpic[i];
+        vpic_info(s);
+        if ( hvm_save_entry(PIC, i, h, s) )
+            return 1;
+    }
+
+    return 0;
 }
 
-static int vpic_load(hvm_domain_context_t *h, void *opaque, int version_id)
+static int vpic_load(struct domain *d, hvm_domain_context_t *h)
 {
-    struct hvm_hw_vpic *s = opaque;
+    struct hvm_hw_vpic *s;
+    uint16_t inst;
     
-    if (version_id != 1)
+    /* Which PIC is this? */
+    inst = hvm_load_instance(h);
+    if ( inst > 1 )
         return -EINVAL;
+    s = &d->arch.hvm_domain.vpic[inst];
 
-    hvm_get_struct(h, s);
-    vpic_info(s);
+    /* Load the state */
+    if ( hvm_load_entry(PIC, h, s) != 0 )
+        return -EINVAL;
 
+    vpic_info(s);
     return 0;
 }
 
+HVM_REGISTER_SAVE_RESTORE(PIC, vpic_save, vpic_load);
+
 void vpic_init(struct domain *d)
 {
     struct hvm_hw_vpic *vpic;
@@ -434,14 +451,12 @@ void vpic_init(struct domain *d)
     memset(vpic, 0, sizeof(*vpic));
     vpic->is_master = 1;
     vpic->elcr      = 1 << 2;
-    hvm_register_savevm(d, "xen_hvm_i8259", 0x20, 1, vpic_save, vpic_load, vpic);
     register_portio_handler(d, 0x20, 2, vpic_intercept_pic_io);
     register_portio_handler(d, 0x4d0, 1, vpic_intercept_elcr_io);
 
     /* Slave PIC. */
     vpic++;
     memset(vpic, 0, sizeof(*vpic));
-    hvm_register_savevm(d, "xen_hvm_i8259", 0xa0, 1, vpic_save, vpic_load, vpic);
     register_portio_handler(d, 0xa0, 2, vpic_intercept_pic_io);
     register_portio_handler(d, 0x4d1, 1, vpic_intercept_elcr_io);
 }
index eda4a3d0288804962e32e5acf771e210158517b6..f0b818462b98ddea4d0789227ef994219c84839b 100644 (file)
 #include <public/hvm/params.h>
 #include <public/hvm/save.h>
 
-typedef void SaveStateHandler(hvm_domain_context_t *h, void *opaque);
-typedef int LoadStateHandler(hvm_domain_context_t *h, void *opaque, int version_id);
-
-#define HVM_SE_IDSTR_LEN 32
-typedef struct HVMStateEntry {
-    char idstr[HVM_SE_IDSTR_LEN];
-    int instance_id;
-    int version_id;
-    SaveStateHandler *save_state;
-    LoadStateHandler *load_state;
-    void *opaque;
-    struct HVMStateEntry *next;
-} HVMStateEntry;
-
 struct hvm_domain {
     unsigned long          shared_page_va;
     unsigned long          buffered_io_va;
@@ -65,7 +51,6 @@ struct hvm_domain {
     uint64_t               params[HVM_NR_PARAMS];
 
     struct hvm_domain_context *hvm_ctxt;
-    HVMStateEntry *first_se;
 };
 
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
index 29ff3226628853666cc0beb00246e120ebbc2400..953cc60b3c01d9ee14615bfb95ddf06288786e6e 100644 (file)
@@ -83,8 +83,8 @@ struct hvm_function_table {
         struct vcpu *v, struct cpu_user_regs *r);
 
     /* save and load hvm guest cpu context for save/restore */
-    void (*save_cpu_ctxt)(hvm_domain_context_t *h, void *opaque);
-    int (*load_cpu_ctxt)(hvm_domain_context_t *h, void *opaque, int version);
+    void (*save_cpu_ctxt)(struct vcpu *v, struct hvm_hw_cpu *ctxt);
+    int (*load_cpu_ctxt)(struct vcpu *v, struct hvm_hw_cpu *ctxt);
 
     /*
      * Examine specifics of the guest state:
index f474756d97ec5772bc7fb722074e2a224d86437b..543fb4dcdb3bf53e1e04867e0bc4490edc853ff1 100644 (file)
@@ -119,133 +119,121 @@ extern unsigned int opt_hvm_debug_level;
 #define TRACE_VMEXIT(index, value)                              \
     current->arch.hvm_vcpu.hvm_trace_values[index] = (value)
 
-/* save/restore support */
-
-//#define HVM_DEBUG_SUSPEND
-
-extern int hvm_register_savevm(struct domain *d,
-                    const char *idstr,
-                    int instance_id,
-                    int version_id,
-                    SaveStateHandler *save_state,
-                    LoadStateHandler *load_state,
-                    void *opaque);
-
-static inline void hvm_ctxt_seek(hvm_domain_context_t *h, unsigned int pos)
-{
-    h->cur = pos;
-}
-
-static inline uint32_t hvm_ctxt_tell(hvm_domain_context_t *h)
-{
-    return h->cur;
-}
-
-static inline int hvm_ctxt_end(hvm_domain_context_t *h)
-{
-    return (h->cur >= h->size || h->cur >= HVM_CTXT_SIZE);
-}
+/*
+ * Save/restore support 
+ */
 
-static inline void hvm_put_byte(hvm_domain_context_t *h, unsigned int i)
+/* Marshalling an entry: check space and fill in the header */
+static inline int _hvm_init_entry(struct hvm_domain_context *h,
+                                  uint16_t tc, uint16_t inst, uint32_t len)
 {
-    if (h->cur >= HVM_CTXT_SIZE) {
-        h->cur++;
-        return;
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( h->size - h->cur < len + sizeof (*d) )
+    {
+        gdprintk(XENLOG_WARNING,
+                 "HVM save: no room for %"PRIu32" + %u bytes "
+                 "for typecode %"PRIu16"\n",
+                 len, (unsigned) sizeof (*d), tc);
+        return -1;
     }
-    h->data[h->cur++] = (char)i;
-}
-
-static inline void hvm_put_8u(hvm_domain_context_t *h, uint8_t b)
-{
-    hvm_put_byte(h, b);
-}
-
-static inline void hvm_put_16u(hvm_domain_context_t *h, uint16_t b)
-{
-    hvm_put_8u(h, b >> 8);
-    hvm_put_8u(h, b);
-}
-
-static inline void hvm_put_32u(hvm_domain_context_t *h, uint32_t b)
-{
-    hvm_put_16u(h, b >> 16);
-    hvm_put_16u(h, b);
-}
-
-static inline void hvm_put_64u(hvm_domain_context_t *h, uint64_t b)
-{
-    hvm_put_32u(h, b >> 32);
-    hvm_put_32u(h, b);
-}
-
-static inline void hvm_put_buffer(hvm_domain_context_t *h, const char *buf, int len)
-{
-    memcpy(&h->data[h->cur], buf, len);
-    h->cur += len;
+    d->typecode = tc;
+    d->instance = inst;
+    d->length = len;
+    h->cur += sizeof (*d);
+    return 0;
 }
 
-static inline char hvm_get_byte(hvm_domain_context_t *h)
+/* Marshalling: copy the contents in a type-safe way */
+#define _hvm_write_entry(_x, _h, _src) do {                     \
+    *(HVM_SAVE_TYPE(_x) *)(&(_h)->data[(_h)->cur]) = *(_src);   \
+    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
+} while (0)
+
+/* Marshalling: init and copy; evaluates to zero on success */
+#define hvm_save_entry(_x, _inst, _h, _src) ({          \
+    int r;                                              \
+    r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x),        \
+                        (_inst), HVM_SAVE_LENGTH(_x));  \
+    if ( r == 0 )                                       \
+        _hvm_write_entry(_x, (_h), (_src));             \
+    r; })
+
+/* Unmarshalling: test an entry's size and typecode and record the instance */
+static inline int _hvm_check_entry(struct hvm_domain_context *h, 
+                                   uint16_t type, uint32_t len)
 {
-    if (h->cur >= HVM_CTXT_SIZE) {
-        printk("hvm_get_byte overflow.\n");
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( len + sizeof (*d) > h->size - h->cur)
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: not enough data left to read %u bytes "
+                 "for type %u\n", len, type);
         return -1;
-    }
-
-    if (h->cur >= h->size) {
-        printk("hvm_get_byte exceed data area.\n");
+    }    
+    if ( type != d->typecode || len != d->length )
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore mismatch: expected type %u length %u, "
+                 "saw type %u length %u\n", type, len, d->typecode, d->length);
         return -1;
     }
-
-    return h->data[h->cur++];
-}
-
-static inline uint8_t hvm_get_8u(hvm_domain_context_t *h)
-{
-    return hvm_get_byte(h);
-}
-
-static inline uint16_t hvm_get_16u(hvm_domain_context_t *h)
-{
-    uint16_t v;
-    v =  hvm_get_8u(h) << 8;
-    v |= hvm_get_8u(h);
-
-    return v;
-}
-
-static inline uint32_t hvm_get_32u(hvm_domain_context_t *h)
-{
-    uint32_t v;
-    v =  hvm_get_16u(h) << 16;
-    v |= hvm_get_16u(h);
-
-    return v;
-}
-
-static inline uint64_t hvm_get_64u(hvm_domain_context_t *h)
-{
-    uint64_t v;
-    v =  (uint64_t)hvm_get_32u(h) << 32;
-    v |= hvm_get_32u(h);
-
-    return v;
+    h->cur += sizeof (*d);
+    return 0;
 }
 
-static inline void hvm_get_buffer(hvm_domain_context_t *h, char *buf, int len)
+/* Unmarshalling: copy the contents in a type-safe way */
+#define _hvm_read_entry(_x, _h, _dst) do {                      \
+    *(_dst) = *(HVM_SAVE_TYPE(_x) *) (&(_h)->data[(_h)->cur]);  \
+    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
+} while (0)
+
+/* Unmarshalling: check, then copy. Evaluates to zero on success. */
+#define hvm_load_entry(_x, _h, _dst) ({                                 \
+    int r;                                                              \
+    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x)); \
+    if ( r == 0 )                                                       \
+        _hvm_read_entry(_x, (_h), (_dst));                              \
+    r; })
+
+/* Unmarshalling: what is the instance ID of the next entry? */
+static inline uint16_t hvm_load_instance(struct hvm_domain_context *h)
 {
-    memcpy(buf, &h->data[h->cur], len);
-    h->cur += len;
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    return d->instance;
 }
 
-#define hvm_put_struct(_h, _p) \
-    hvm_put_buffer((_h), (char *)(_p), sizeof(*(_p)))
-#define hvm_get_struct(_h, _p) \
-    hvm_get_buffer((_h), (char *)(_p), sizeof(*(_p)))
-
+/* Handler types for different types of save-file entry. 
+ * The save handler may save multiple instances of a type into the buffer;
+ * the load handler will be called once for each instance found when
+ * restoring.  Both return non-zero on error. */
+typedef int (*hvm_save_handler) (struct domain *d, 
+                                 hvm_domain_context_t *h);
+typedef int (*hvm_load_handler) (struct domain *d,
+                                 hvm_domain_context_t *h);
+
+/* Init-time function to declare a pair of handlers for a type */
+void hvm_register_savevm(uint16_t typecode, 
+                         hvm_save_handler save_state,
+                         hvm_load_handler load_state);
+
+/* Syntactic sugar around that function */
+#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _load)             \
+static int __hvm_register_##_x##_save_and_restore(void)         \
+{                                                               \
+    hvm_register_savevm(HVM_SAVE_CODE(_x), &_save, &_load);     \
+    return 0;                                                   \
+}                                                               \
+__initcall(__hvm_register_##_x##_save_and_restore);
+
+
+/* Entry points for saving and restoring HVM domain state */
 int hvm_save(struct domain *d, hvm_domain_context_t *h);
 int hvm_load(struct domain *d, hvm_domain_context_t *h);
 
-void shpage_init(struct domain *d, shared_iopage_t *sp);
+/* End of save/restore */
 
 extern char hvm_io_bitmap[];
 extern int hvm_enabled;
index 9bd3e05667ed0a2828f090a97aeceee6f04991b2..22e1f4d19628807d27370fdf9a1b674fac6f9d9a 100644 (file)
  */
 
 /* 
- * Save/restore header
+ * Each entry is preceded by a descriptor giving its type and length
  */
+struct hvm_save_descriptor {
+    uint16_t typecode;          /* Used to demux the various types below */
+    uint16_t instance;          /* Further demux within a type */
+    uint32_t length;            /* In bytes, *not* including this descriptor */
+};
+
+
+/* 
+ * Each entry has a datatype associated with it: for example, the CPU state 
+ * is saved as a HVM_SAVE_TYPE(CPU), which has HVM_SAVE_LENGTH(CPU), 
+ * and is identified by a descriptor with typecode HVM_SAVE_CODE(CPU).
+ * DECLARE_HVM_SAVE_TYPE binds these things together with some type-system
+ * ugliness.
+ */
+
+#define DECLARE_HVM_SAVE_TYPE(_x, _code, _type)                   \
+  struct __HVM_SAVE_TYPE_##_x { _type t; char c[_code]; }
+
+#define HVM_SAVE_TYPE(_x) typeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->t)
+#define HVM_SAVE_LENGTH(_x) (sizeof (HVM_SAVE_TYPE(_x)))
+#define HVM_SAVE_CODE(_x) (sizeof (((struct __HVM_SAVE_TYPE_##_x *)(0))->c))
+
 
-#define HVM_SAVE_TYPE_HEADER 0
+/* 
+ * Save/restore header: general info about the save file. 
+ */
 
 #define HVM_FILE_MAGIC   0x54381286
 #define HVM_FILE_VERSION 0x00000001
 
 struct hvm_save_header {
-    uint32_t magic;
-    uint32_t version;
-    uint32_t cpuid;
+    uint32_t magic;             /* Must be HVM_FILE_MAGIC */
+    uint32_t version;           /* File format version */
+    uint64_t changeset;         /* Version of Xen that saved this file */
+    uint32_t cpuid;             /* CPUID[0x01][%eax] on the saving machine */
 };
 
+DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header);
+
+
 /*
  * Processor
  */
-#define HVM_SAVE_TYPE_CPU  1
+
 struct hvm_hw_cpu {
     uint64_t eip;
     uint64_t esp;
@@ -124,11 +152,13 @@ struct hvm_hw_cpu {
     uint64_t tsc;
 };
 
+DECLARE_HVM_SAVE_TYPE(CPU, 2, struct hvm_hw_cpu);
+
 
 /* 
  *  PIT
  */
-#define HVM_SAVE_TYPE_PIT 2
+
 struct hvm_hw_pit {
     struct hvm_hw_pit_channel {
         int64_t count_load_time;
@@ -148,11 +178,13 @@ struct hvm_hw_pit {
     uint32_t speaker_data_on;
 };
 
+DECLARE_HVM_SAVE_TYPE(PIT, 3, struct hvm_hw_pit);
+
 
 /*
  * PIC
  */
-#define HVM_SAVE_TYPE_PIC 3
+
 struct hvm_hw_vpic {
     /* IR line bitmasks. */
     uint8_t irr;
@@ -201,11 +233,12 @@ struct hvm_hw_vpic {
     uint8_t int_output;
 };
 
+DECLARE_HVM_SAVE_TYPE(PIC, 4, struct hvm_hw_vpic);
+
 
 /*
  * IO-APIC
  */
-#define HVM_SAVE_TYPE_IOAPIC 4
 
 #ifdef __ia64__
 #define VIOAPIC_IS_IOSAPIC 1
@@ -242,11 +275,13 @@ struct hvm_hw_vioapic {
     } redirtbl[VIOAPIC_NUM_PINS];
 };
 
+DECLARE_HVM_SAVE_TYPE(IOAPIC, 5, struct hvm_hw_vioapic);
+
 
 /*
  * IRQ
  */
-#define HVM_SAVE_TYPE_IRQ 5
+
 struct hvm_hw_irq {
     /*
      * Virtual interrupt wires for a single PCI bus.
@@ -309,22 +344,40 @@ struct hvm_hw_irq {
     u8 round_robin_prev_vcpu;
 };
 
+DECLARE_HVM_SAVE_TYPE(IRQ, 6, struct hvm_hw_irq);
 
 /*
  * LAPIC
  */
-#define HVM_SAVE_TYPE_LAPIC 6
+
 struct hvm_hw_lapic {
     uint64_t             apic_base_msr;
     uint32_t             disabled; /* VLAPIC_xx_DISABLED */
     uint32_t             timer_divisor;
 };
 
-#define HVM_SAVE_TYPE_LAPIC_REGS 7
+DECLARE_HVM_SAVE_TYPE(LAPIC, 7, struct hvm_hw_lapic);
 
 struct hvm_hw_lapic_regs {
     /* A 4k page of register state */
     uint8_t  data[0x400];
 };
 
+DECLARE_HVM_SAVE_TYPE(LAPIC_REGS, 8, struct hvm_hw_lapic_regs);
+
+
+/* 
+ * Largest type-code in use
+ */
+#define HVM_SAVE_CODE_MAX 8
+
+
+/* 
+ * The series of save records is teminated by a zero-type, zero-length 
+ * descriptor.
+ */
+
+struct hvm_save_end {};
+DECLARE_HVM_SAVE_TYPE(END, 0, struct hvm_save_end);
+
 #endif /* __XEN_PUBLIC_HVM_SAVE_H__ */